Restructure cargo_test code and introduce CargoTestError

diff --git a/src/bin/bench.rs b/src/bin/bench.rs
index c90bdba2dd02843021f55894a3f619d1fe57f5a4..655fbff60e5c757f6b63d679cbd8b01479bde372 100644 (file)
--- a/src/bin/bench.rs
+++ b/src/bin/bench.rs
@@ -95,7 +95,10 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
     match err {
         None => Ok(None),
         Some(err) => {
-            Err(CliError::from_error(Human(err), 101))
+            Err(match err.exit.as_ref().and_then(|e| e.code()) {
+                Some(i) => CliError::new("", i),
+                None => CliError::from_error(Human(err), 101)
+            })
         }
     }
 }

diff --git a/src/bin/test.rs b/src/bin/test.rs
index 424e8519cc316facec803fb934d806b0fcf0ae7a..ade460165289000a93ed476c8325a31f3494d0c7 100644 (file)
--- a/src/bin/test.rs
+++ b/src/bin/test.rs
@@ -101,7 +101,10 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
     match err {
         None => Ok(None),
         Some(err) => {
-            Err(CliError::from_error(Human(err), 101))
+            Err(match err.exit.as_ref().and_then(|e| e.code()) {
+                Some(i) => CliError::new("", i),
+                None => CliError::from_error(Human(err), 101)
+            })
         }
     }
 }

diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs
index 6c7227cc285e9a8285f0dbb1ab62daaecf9bf7c9..6370f5c02ad5851698ad35e8e173a4d5850420b6 100644 (file)
--- a/src/cargo/ops/cargo_test.rs
+++ b/src/cargo/ops/cargo_test.rs
@@ -4,7 +4,7 @@ use std::path::Path;
 use core::Source;
 use sources::PathSource;
 use ops::{self, ExecEngine, ProcessEngine, Compilation};
-use util::{self, CargoResult, CargoError, internal};
+use util::{self, CargoResult, CargoTestError, ProcessError};
 
 pub struct TestOptions<'a> {
     pub compile_opts: ops::CompileOptions<'a>,
@@ -12,47 +12,124 @@ pub struct TestOptions<'a> {
     pub no_fail_fast: bool,
 }
 
+
+
 #[allow(deprecated)] // connect => join in 1.3
 pub fn run_tests(manifest_path: &Path,
                  options: &TestOptions,
-                 test_args: &[String]) -> CargoResult<Option<Box<CargoError>>> {
-    let config = options.compile_opts.config;
-    let (compile, error) = try!(build_and_run(manifest_path, options, test_args));
+                 test_args: &[String]) -> CargoResult<Option<CargoTestError>> {
+    let compilation = try!(compile_tests(manifest_path, options));
+
+    if options.no_run {
+        return Ok(None)
+    }
+    let mut errors = try!(run_unit_tests(options, test_args, &compilation));
 
     // If we have an error and want to fail fast, return
-    if error.is_some() && !options.no_fail_fast {
-        return Ok(error)
+    if errors.len() > 0 && !options.no_fail_fast {
+        return Ok(Some(CargoTestError::from(&errors[..])))
     }
+
     // If a specific test was requested or we're not running any tests at all,
     // don't run any doc tests.
     if let ops::CompileFilter::Only { .. } = options.compile_opts.filter {
-        return Ok(None)
+        match errors.len() {
+            0 => return Ok(None),
+            _ => return Ok(Some(CargoTestError::from(&errors[..])))
+        }
     }
-    if options.no_run {
-        return Ok(None)
+
+    errors.extend(try!(run_doc_tests(options, test_args, &compilation)));
+    if errors.len() == 0 {
+        Ok(None)
+    } else {
+        Ok(Some(CargoTestError::from(&errors[..])))
+    }
+}
+
+pub fn run_benches(manifest_path: &Path,
+                   options: &TestOptions,
+                   args: &[String]) -> CargoResult<Option<CargoTestError>> {
+    let mut args = args.to_vec();
+    args.push("--bench".to_string());
+    let compilation = try!(compile_tests(manifest_path, options));
+    let errors = try!(run_unit_tests(options, &args, &compilation));
+    match errors.len() {
+        0 => Ok(None),
+        _ => Ok(Some(CargoTestError::from(&errors[..]))),
     }
+}
 
-    let libs = compile.package.targets().iter()
-                      .filter(|t| t.doctested())
-                      .map(|t| (t.src_path(), t.name(), t.crate_name()));
+fn compile_tests<'a>(manifest_path: &Path,
+                     options: &TestOptions<'a>)
+                     -> CargoResult<Compilation<'a>> {
+    let config = options.compile_opts.config;
+    let mut source = try!(PathSource::for_path(&manifest_path.parent().unwrap(),
+                                               config));
+    try!(source.update());
+
+    let mut compilation = try!(ops::compile(manifest_path, &options.compile_opts));
+    compilation.tests.sort();
+    Ok(compilation)
+}
+
+/// Run the unit and integration tests of a project.
+fn run_unit_tests(options: &TestOptions,
+                  test_args: &[String],
+                  compilation: &Compilation)
+                  -> CargoResult<Vec<ProcessError>> {
+    let config = options.compile_opts.config;
+    let cwd = options.compile_opts.config.cwd();
+
+    let mut errors = Vec::new();
+
+    for &(_, ref exe) in &compilation.tests {
+        let to_display = match util::without_prefix(exe, &cwd) {
+            Some(path) => path,
+            None => &**exe,
+        };
+        let mut cmd = try!(compilation.target_process(exe, &compilation.package));
+        cmd.args(test_args);
+        try!(config.shell().concise(|shell| {
+            shell.status("Running", to_display.display().to_string())
+        }));
+        try!(config.shell().verbose(|shell| {
+            shell.status("Running", cmd.to_string())
+        }));
+
+        if let Err(e) = ExecEngine::exec(&mut ProcessEngine, cmd) {
+            errors.push(e);
+            if !options.no_fail_fast {
+                break
+            }
+        }
+    }
+    Ok(errors)
+}
 
-    let mut errors = match error {
-        None => Vec::new(),
-        Some(err) => vec![err],
-    };
+#[allow(deprecated)] // connect => join in 1.3
+fn run_doc_tests(options: &TestOptions,
+                 test_args: &[String],
+                 compilation: &Compilation)
+                 -> CargoResult<Vec<ProcessError>> {
+    let mut errors = Vec::new();
+    let config = options.compile_opts.config;
+    let libs = compilation.package.targets().iter()
+                    .filter(|t| t.doctested())
+                    .map(|t| (t.src_path(), t.name(), t.crate_name()));
     for (lib, name, crate_name) in libs {
         try!(config.shell().status("Doc-tests", name));
-        let mut p = try!(compile.rustdoc_process(&compile.package));
+        let mut p = try!(compilation.rustdoc_process(&compilation.package));
         p.arg("--test").arg(lib)
          .arg("--crate-name").arg(&crate_name)
-         .cwd(compile.package.root());
+         .cwd(compilation.package.root());
 
-        for &rust_dep in &[&compile.deps_output, &compile.root_output] {
+        for &rust_dep in &[&compilation.deps_output, &compilation.root_output] {
             let mut arg = OsString::from("dependency=");
             arg.push(rust_dep);
             p.arg("-L").arg(arg);
         }
-        for native_dep in compile.native_dirs.values() {
+        for native_dep in compilation.native_dirs.values() {
             p.arg("-L").arg(native_dep);
         }
 
@@ -60,11 +137,11 @@ pub fn run_tests(manifest_path: &Path,
             p.arg("--test-args").arg(&test_args.connect(" "));
         }
 
-        for feat in compile.features.iter() {
+        for feat in compilation.features.iter() {
             p.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
         }
 
-        for (_, libs) in compile.libraries.iter() {
+        for (_, libs) in compilation.libraries.iter() {
             for &(ref target, ref lib) in libs.iter() {
                 // Note that we can *only* doctest rlib outputs here.  A
                 // staticlib output cannot be linked by the compiler (it just
@@ -91,81 +168,11 @@ pub fn run_tests(manifest_path: &Path,
             shell.status("Running", p.to_string())
         }));
         if let Err(e) = ExecEngine::exec(&mut ProcessEngine, p) {
-            errors.push(Box::new(e));
-            if !options.no_fail_fast {
-                break
-            }
-        }
-    }
-    if errors.is_empty() {
-        Ok(None)
-    } else if errors.len() == 1 {
-        Ok(Some(errors.pop().unwrap()))
-    } else {
-        let err = internal("Multiple tests failed");
-        Ok(Some(err))
-    }
-
-}
-
-pub fn run_benches(manifest_path: &Path,
-                   options: &TestOptions,
-                   args: &[String]) -> CargoResult<Option<Box<CargoError>>> {
-    let mut args = args.to_vec();
-    args.push("--bench".to_string());
-
-    match try!(build_and_run(manifest_path, options, &args)).1 {
-        Some(err) => Ok(Some(err)),
-        None => Ok(None)
-    }
-}
-
-// This function always has to return the Compilation, regardless of errors.
-// Otherwise --no-fail-fast is not possible.
-fn build_and_run<'a>(manifest_path: &Path,
-                     options: &TestOptions<'a>,
-                     test_args: &[String])
-                     -> CargoResult<(Compilation<'a>, Option<Box<CargoError>>)> {
-    let config = options.compile_opts.config;
-    let mut source = try!(PathSource::for_path(&manifest_path.parent().unwrap(),
-                                               config));
-    try!(source.update());
-
-    let mut compile = try!(ops::compile(manifest_path, &options.compile_opts));
-    if options.no_run { return Ok((compile, None)) }
-    compile.tests.sort();
-
-    let cwd = config.cwd();
-    let mut errors = Vec::new();
-    for &(_, ref exe) in &compile.tests {
-        let to_display = match util::without_prefix(exe, &cwd) {
-            Some(path) => path,
-            None => &**exe,
-        };
-        let mut cmd = try!(compile.target_process(exe, &compile.package));
-        cmd.args(test_args);
-        try!(config.shell().concise(|shell| {
-            shell.status("Running", to_display.display().to_string())
-        }));
-        try!(config.shell().verbose(|shell| {
-            shell.status("Running", cmd.to_string())
-        }));
-
-        if let Err(e) = ExecEngine::exec(&mut ProcessEngine, cmd) {
             errors.push(e);
             if !options.no_fail_fast {
                 break
             }
         }
     }
-    if errors.is_empty() {
-        Ok((compile, None))
-    } else if errors.len() == 1 {
-        // Just one error occured => we can return it.
-        Ok((compile, Some(Box::new(errors.pop().unwrap()))))
-    } else {
-        // Multiple tests failed => Create a more generic error
-        let err = internal("Multiple tests failed");
-        Ok((compile, Some(err)))
-    }
+    Ok(errors)
 }

diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs
index 4c24b34c59b8d0fd18c7a013929460ee2c240b44..622e09c1ed2718c49b4a1dc18e775ad1e4019c9c 100644 (file)
--- a/src/cargo/util/errors.rs
+++ b/src/cargo/util/errors.rs
@@ -131,6 +131,50 @@ impl fmt::Debug for ProcessError {
     }
 }
 
+// =============================================================================
+// Cargo test errors.
+
+/// Error when testcases fail
+pub struct CargoTestError {
+    pub desc: String,
+    pub exit: Option<ExitStatus>,
+    cause: Option<io::Error>,
+}
+
+impl fmt::Display for CargoTestError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.desc, f)
+    }
+}
+
+impl fmt::Debug for CargoTestError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(self, f)
+    }
+}
+
+impl Error for CargoTestError {
+    fn description(&self) -> &str { &self.desc }
+    #[allow(trivial_casts)]
+    fn cause(&self) -> Option<&Error> {
+        self.cause.as_ref().map(|s| s as &Error)
+    }
+}
+
+#[allow(deprecated)] // connect => join in 1.3
+impl<'a> From<&'a [ProcessError]> for CargoTestError {
+    fn from(errors: &[ProcessError]) -> Self {
+        if errors.len() == 0 { panic!("Cannot create CargoTestError from empty Vec") }
+        let desc = errors.iter().map(|error| error.desc.clone()).collect::<Vec<String>>().connect("\n");
+        CargoTestError {
+            desc: desc,
+            exit: errors[0].exit,
+            cause: None,
+        }
+    }
+}
+
+
 // =============================================================================
 // Concrete errors
 
@@ -274,6 +318,7 @@ impl CargoError for git2::Error {}
 impl CargoError for json::DecoderError {}
 impl CargoError for curl::ErrCode {}
 impl CargoError for ProcessError {}
+impl CargoError for CargoTestError {}
 impl CargoError for CliError {}
 impl CargoError for toml::Error {}
 impl CargoError for toml::DecodeError {}

diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs
index b47e8095dd4cd0dea4e23d1dd5041a5aad964243..95b01a7239539a920fc7a2f8a2868d64acd7f4a2 100644 (file)
--- a/src/cargo/util/mod.rs
+++ b/src/cargo/util/mod.rs
@@ -2,7 +2,7 @@ pub use self::config::Config;
 pub use self::dependency_queue::Dependency;
 pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness};
 pub use self::errors::{CargoResult, CargoError, ChainError, CliResult};
-pub use self::errors::{CliError, ProcessError};
+pub use self::errors::{CliError, ProcessError, CargoTestError};
 pub use self::errors::{Human, caused_human};
 pub use self::errors::{process_error, internal_error, internal, human};
 pub use self::graph::Graph;